From f942d8d4313ae63240e1155be0cac40ec3ba34ba Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 18 Aug 2017 19:37:12 +0200 Subject: [PATCH] Using required dep as a feature is a warning for now, not an error --- src/cargo/core/resolver/mod.rs | 29 ++++++++++++++++++++---- src/cargo/ops/cargo_generate_lockfile.rs | 5 ++-- src/cargo/ops/resolve.rs | 23 +++++++++++++------ tests/features.rs | 23 ++++++++++++------- tests/resolve.rs | 2 +- 5 files changed, 59 insertions(+), 23 deletions(-) diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 80ff03050..2b751d6a8 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -57,6 +57,7 @@ use url::Url; use core::{PackageId, Registry, SourceId, Summary, Dependency}; use core::PackageIdSpec; +use core::shell::Shell; use util::Graph; use util::errors::{CargoResult, CargoError}; use util::profile; @@ -330,6 +331,9 @@ struct Context<'a> { resolve_replacements: RcList<(PackageId, PackageId)>, replacements: &'a [(PackageIdSpec, Dependency)], + + // These warnings are printed after resolution. + warnings: RcList, } type Activations = HashMap>>; @@ -337,13 +341,15 @@ type Activations = HashMap>>; /// Builds the list of all packages required to build the first argument. pub fn resolve(summaries: &[(Summary, Method)], replacements: &[(PackageIdSpec, Dependency)], - registry: &mut Registry) -> CargoResult { + registry: &mut Registry, + shell: Option<&mut Shell>) -> CargoResult { let cx = Context { resolve_graph: RcList::new(), resolve_features: HashMap::new(), resolve_replacements: RcList::new(), activations: HashMap::new(), replacements: replacements, + warnings: RcList::new(), }; let _p = profile::start(format!("resolving")); let cx = activate_deps_loop(cx, registry, summaries)?; @@ -368,8 +374,17 @@ pub fn resolve(summaries: &[(Summary, Method)], } check_cycles(&resolve, &cx.activations)?; - trace!("resolved: {:?}", resolve); + + // If we have a shell, emit warnings about required deps used as feature. + if let Some(shell) = shell { + let mut warnings = &cx.warnings; + while let Some(ref head) = warnings.head { + shell.warn(&head.0)?; + warnings = &head.1; + } + } + Ok(resolve) } @@ -1088,9 +1103,13 @@ impl<'a> Context<'a> { // to `ret`. let base = feature_deps.remove(dep.name()).unwrap_or((false, vec![])); if !dep.is_optional() && base.0 { - bail!("Package `{}` does not have feature `{}`. It has a required dependency with \ - that name, but only optional dependencies can be used as features.", - s.package_id(), dep.name()); + self.warnings.push( + format!("Package `{}` does not have feature `{}`. It has a required dependency \ + with that name, but only optional dependencies can be used as features. \ + This is currently a warning to ease the transition, but it will become an \ + error in the future.", + s.package_id(), dep.name()) + ); } let mut base = base.1; base.extend(dep.features().iter().cloned()); diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 510f3d1c6..29c3749c6 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -19,7 +19,7 @@ pub fn generate_lockfile(ws: &Workspace) -> CargoResult<()> { let mut registry = PackageRegistry::new(ws.config())?; let resolve = ops::resolve_with_previous(&mut registry, ws, Method::Everything, - None, None, &[])?; + None, None, &[], true)?; ops::write_pkg_lockfile(ws, &resolve)?; Ok(()) } @@ -79,7 +79,8 @@ pub fn update_lockfile(ws: &Workspace, opts: &UpdateOptions) Method::Everything, Some(&previous_resolve), Some(&to_avoid), - &[])?; + &[], + true)?; // Summarize what is changing for the user. let print_change = |status: &str, msg: String| { diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index d4387e49a..074f79898 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -15,7 +15,7 @@ use util::errors::{CargoResult, CargoResultExt}; /// lockfile. pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolve)> { let mut registry = PackageRegistry::new(ws.config())?; - let resolve = resolve_with_registry(ws, &mut registry)?; + let resolve = resolve_with_registry(ws, &mut registry, true)?; let packages = get_resolved_packages(&resolve, registry); Ok((packages, resolve)) } @@ -44,7 +44,7 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>, let resolve = if ws.require_optional_deps() { // First, resolve the root_package's *listed* dependencies, as well as // downloading and updating all remotes and such. - let resolve = resolve_with_registry(ws, &mut registry)?; + let resolve = resolve_with_registry(ws, &mut registry, false)?; // Second, resolve with precisely what we're doing. Filter out // transitive dependencies if necessary, specify features, handle @@ -79,19 +79,19 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>, let resolved_with_overrides = ops::resolve_with_previous(&mut registry, ws, method, resolve.as_ref(), None, - specs)?; + specs, true)?; let packages = get_resolved_packages(&resolved_with_overrides, registry); Ok((packages, resolved_with_overrides)) } -fn resolve_with_registry(ws: &Workspace, registry: &mut PackageRegistry) +fn resolve_with_registry(ws: &Workspace, registry: &mut PackageRegistry, warn: bool) -> CargoResult { let prev = ops::load_pkg_lockfile(ws)?; let resolve = resolve_with_previous(registry, ws, Method::Everything, - prev.as_ref(), None, &[])?; + prev.as_ref(), None, &[], warn)?; if !ws.is_ephemeral() { ops::write_pkg_lockfile(ws, &resolve)?; @@ -114,7 +114,8 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry, method: Method, previous: Option<&'a Resolve>, to_avoid: Option<&HashSet<&'a PackageId>>, - specs: &[PackageIdSpec]) + specs: &[PackageIdSpec], + warn: bool) -> CargoResult { // Here we place an artificial limitation that all non-registry sources // cannot be locked at more than one revision. This means that if a git @@ -256,9 +257,17 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry, None => root_replace.to_vec(), }; + let mut shell; + let opt_shell = if warn { + shell = ws.config().shell(); + Some(&mut *shell) + } else { + None + }; let mut resolved = resolver::resolve(&summaries, &replace, - registry)?; + registry, + opt_shell)?; resolved.register_used_patches(registry.patches()); if let Some(previous) = previous { resolved.merge_from(previous)?; diff --git a/tests/features.rs b/tests/features.rs index 44cbf9610..7aec1b99e 100644 --- a/tests/features.rs +++ b/tests/features.rs @@ -237,7 +237,7 @@ fn invalid9() { [dependencies.bar] path = "bar" "#) - .file("src/main.rs", "") + .file("src/main.rs", "fn main() {}") .file("bar/Cargo.toml", r#" [package] name = "bar" @@ -247,9 +247,12 @@ fn invalid9() { .file("bar/src/lib.rs", ""); assert_that(p.cargo_process("build").arg("--features").arg("bar"), - execs().with_status(101).with_stderr("\ -[ERROR] Package `foo v0.0.1 ([..])` does not have feature `bar`. It has a required dependency with \ -that name, but only optional dependencies can be used as features. + execs().with_status(0).with_stderr("\ +warning: Package `foo v0.0.1 ([..])` does not have feature `bar`. It has a required dependency with \ +that name, but only optional dependencies can be used as features. [..] + Compiling bar v0.0.1 ([..]) + Compiling foo v0.0.1 ([..]) + Finished dev [unoptimized + debuginfo] target(s) in [..] secs ")); } @@ -266,7 +269,7 @@ fn invalid10() { path = "bar" features = ["baz"] "#) - .file("src/main.rs", "") + .file("src/main.rs", "fn main() {}") .file("bar/Cargo.toml", r#" [package] name = "bar" @@ -286,9 +289,13 @@ fn invalid10() { .file("bar/baz/src/lib.rs", ""); assert_that(p.cargo_process("build"), - execs().with_status(101).with_stderr("\ -[ERROR] Package `bar v0.0.1 ([..])` does not have feature `baz`. It has a required dependency with \ -that name, but only optional dependencies can be used as features. + execs().with_status(0).with_stderr("\ +warning: Package `bar v0.0.1 ([..])` does not have feature `baz`. It has a required dependency with \ +that name, but only optional dependencies can be used as features. [..] + Compiling baz v0.0.1 ([..]) + Compiling bar v0.0.1 ([..]) + Compiling foo v0.0.1 ([..]) + Finished dev [unoptimized + debuginfo] target(s) in [..] secs ")); } diff --git a/tests/resolve.rs b/tests/resolve.rs index 16395a255..9b8ccb0a2 100644 --- a/tests/resolve.rs +++ b/tests/resolve.rs @@ -32,7 +32,7 @@ fn resolve(pkg: PackageId, deps: Vec, registry: &[Summary]) let mut registry = MyRegistry(registry); let summary = Summary::new(pkg.clone(), deps, HashMap::new()).unwrap(); let method = Method::Everything; - let resolve = resolver::resolve(&[(summary, method)], &[], &mut registry)?; + let resolve = resolver::resolve(&[(summary, method)], &[], &mut registry, None)?; let res = resolve.iter().cloned().collect(); Ok(res) } -- 2.30.2